-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ZIR-304: support Zhlt::BackOp in picus mode #139
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome start!
zirgen/compiler/picus/picus.cpp
Outdated
size_t distance = back.getDistance().getZExtValue(); | ||
AnySignal signal = signalize(freshName(), back.getType()); | ||
if (distance > 0) { | ||
declareSignals(signal, /*isInput=*/true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I told you the wrong thing here. Backs shouldn't really be "inputs" because they aren't part of the call interface — doing that would mean we need to have corresponding arguments when we generate calls to the module. I asked Shankara to add a new predicate called assume-deterministic
that marks a signal as deterministic without making it part of the call interface. This probably calls for a change to declareSignals
. Here's a test case Shankara and I discussed:
Input:
component Foo(reg: NondetReg) {
Reg(r@1)
}
Expected output (probably):
(prime-number 2013265921)
(begin-module Foo)
(input reg_layout_super)
(input reg_super)
(output layout_super_super)
(output result_super_super_super)
(output result_super_reg_super)
(assume-deterministic reg_back1)
(call [layout_super_super result_super_super_super result_super_reg_super] Reg [reg_back1])
(end-module)
zirgen/compiler/picus/test/back.zir
Outdated
|
||
// CHECK: (prime-number 2013265921) | ||
// CHECK: (begin-module Count) | ||
// CHECK: (input x0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the full output for this test, x0
corresponds to the argument first: Val
rather than the back. I think we should make a filecheck assertion about all of the inputs and signals corresponding to the back (x1_*
).
3e1c083
to
33dc293
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's in here is sound and reasonable, though it looks like the case of zero distance backs still isn't handled. I'd be okay to merge what's in here once CI passes, though.
a2b0a69
to
cb1eeae
Compare
No description provided.